[FRAME] Simple multi block migrations #14275

ggwpez · 2023-05-31T12:15:05Z

Partial paritytech/polkadot-sdk#198 (on_poll in #14279).

`pallet-migrations`

This pallet takes a configuration of migrations Vec<Box<dyn SteppedMigration<…>>> and executes these in order on_initialize. The execution starts on each runtime upgrade. The unique identifiers of each executed migration are stored to prevent accidental re-execution of old ones.
While migrations are executed, only Mandatory extrinsics and inherents are allowed to execute. This is accomplished by implementing MultiStepMigrator for the pallet and querying it in frame-executive before each non-mandatory TX. Concretely this means that these transactions will panic and make a block un-importable. The BlockBuilder respects this limitation and retains all extrinsics in the pool while migrations are ongoing. Once the MBMs are done, these extrinsics will start to be included again. From a user's perspective this just results in an increased time until the TX is included.

The pallet supports an UpgradeStatusHandler that can be used to notify external logic of upgrade start/finish (for example to pause XCM dispatch).

Error recovery is very limited in the case that a migration errors or times out (exceeds its max_steps). Currently the runtime dev can decide in UpgradeStatusHandler::failed how to handle this. One follow-up would be to pair this with the SafeMode pallet and enact safe mode when an upgrade fails, to allow governance to rescue the chain. This is currently not possible, since governance is not Mandatory.

`frame-executive` + `BlockBuilder`

Executive now additionally takes an MultiStepMigrator trait that defaults to ().
IT IS PARAMOUNT TO SET THIS TO THE MIGRATIONS PALLET WHEN YOU DEPLOY IT.
The kitchensink runtime contains an example.

The block builder is aware of the new after_inherents function and uses it to figure out the MBM state.
As comparison the old and new logic (changes with +):

Current order:
	on_runtime_upgrade
	System::initialize
	on_initialize
	apply inherents
	apply extrinsics
	on_idle
	on_finalize

New order:
	on_runtime_upgrade
	System::initialize
	on_initialize
	apply inherents
+	after_inherents:
+		progress mbms
+		poll TBD
+	If not MBM:
+		apply extrinsics
+		on_idle
	on_finalize

Runtime API

BlockBuilder: Added after_inherents to progress ongoing MBMs and the poll hook TBD in FRAME: Add new poll hook + various other improvements to pallet hooks #14279.

`frame-support`

SteppedMigration: A multi-step migration.
ExtrinsicSuspenderQuery: Implemented by the migrations pallet and used by frame-executive to check TX suspension.
UpgradeStatusHandler: React to upgrade start, completion and failure.
FailedUpgradeHandling: Decide how to handle a failed upgrade. Either ForceUnstuck or KeepStuck.

TODO

Consume weight correctly
Cleanup

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

…ation

frame/executive/src/lib.rs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

frame/support/src/migrations.rs

kianenigma · 2023-05-31T17:25:35Z

frame/support/src/migrations.rs

+
+/// A migration that can proceed in multiple steps.
+pub trait SteppedMigration {
+	fn id(&self) -> BoundedVec<u8, ConstU32<32>>;


Vec is also fine here, but I have usually seen types like [8u; 32] being used for such cases.

I also made it generic. But with the new config defaults, we could add some sane default like this 🤔

kianenigma

So far everything LGTM.

Can fn poll() be done in a separate PR? I presume yes, and perhaps it would be good for someone to start that already.

One important topic related to https://github.com/paritytech/substrate/pull/14275/files#r1211972428 is also tx-pool validation. @ggwpez and I discussed if fn validate_transaction should also be aware of transaction suspension, and signal the pool that everything is "temporarily" invalid.

liamaharon · 2023-05-31T19:33:53Z

@kianenigma am I right to assume we'll want to make it easy to test these with try-runtime-cli? Ideally should be as easy as on-runtime-upgrade, maybe make it auto-mine blocks until all multi-block migrations are complete & provide a hook that runs afterwards?

xlc · 2023-05-31T21:09:55Z

Does this pauses user tx and incoming XCM etc? because otherwise it will be bad.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez · 2023-06-01T12:59:31Z

Does this pauses user tx and incoming XCM etc? because otherwise it will be bad.

It pauses non-mandatory extrinsics in frame-executive by checking a flag. For XCM we probably need a suspension and resume hook additionally.

But now that Im thinking about this again; this would make it impossible to rescue if one of the migrations errors, since governance requires things like the referenda pallet.
So maybe in the case that something errors, it could be put into SafeMode to re-allow few extrinsics.

kianenigma · 2023-06-01T14:41:17Z

@kianenigma am I right to assume we'll want to make it easy to test these with try-runtime-cli? Ideally should be as easy as on-runtime-upgrade, maybe make it auto-mine blocks until all multi-block migrations are complete & provide a hook that runs afterwards?

Good to think about it early, and yes I think it is possible with little effort. I believe follow-chain will already do everything needed to test an MBM?

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

paritytech-cicd-pr · 2023-06-29T15:25:00Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3092555

paritytech-cicd-pr · 2023-06-29T15:25:00Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3092556

kianenigma · 2023-07-20T14:02:38Z

Is this more or less ready for final review?

ggwpez · 2023-07-20T14:06:25Z

Still first need to merge #14414. I will ping you there for review when its ready.

ggwpez added 7 commits May 30, 2023 11:50

Add stepped migrations and pallet

dc35acf

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup

283d51c

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Comment

7c53a74

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Suspension and benches

edb005c

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Add generic to Executive

8331fbe

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup

c23259f

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Merge remote-tracking branch 'origin/master' into oty-multi-step-migr…

9ffd3d7

…ation

ggwpez commented May 31, 2023

View reviewed changes

frame/executive/src/lib.rs Outdated Show resolved Hide resolved

Add Executed

3afdf0a

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez marked this pull request as ready for review May 31, 2023 16:29

ggwpez requested review from a team and kianenigma May 31, 2023 16:29

ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label May 31, 2023

ggwpez self-assigned this May 31, 2023

ggwpez added 2 commits May 31, 2023 18:32

Add events

104fd6d

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fixes

1e18cae

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma reviewed May 31, 2023

View reviewed changes

frame/support/src/migrations.rs Outdated Show resolved Hide resolved

paritytech-ci requested a review from a team May 31, 2023 17:24

kianenigma reviewed May 31, 2023

View reviewed changes

paritytech-ci requested a review from a team May 31, 2023 17:25

kianenigma reviewed May 31, 2023

View reviewed changes

paritytech-ci requested a review from a team May 31, 2023 17:30

kianenigma mentioned this pull request May 31, 2023

FRAME: Add new poll hook + various other improvements to pallet hooks #14279

Draft

4 tasks

Make cursor generic

7ab621a

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix kitchensink

acd99bb

liamaharon self-requested a review June 15, 2023 17:03

ggwpez added 6 commits June 16, 2023 13:22

Add log target

7435788

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Factor out apply_inherents

6b1d6b1

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Factor out apply_extrinsics

0b7dddc

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Factor out print_summary

648e005

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Pimp print_summary

24c8bf6

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Cleanup

4b99c6e

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez mentioned this pull request Jun 16, 2023

Refactor BlockBuilder::propose_with #14405

Merged

ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Jun 16, 2023

ggwpez and others added 10 commits June 16, 2023 14:08

Fixes

93f0d32

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Backup

c57bbf5

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Merge branch 'oty-refactor-block-builder' into oty-multi-step-migration

30c302e

Fix stuff

4101e82

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Works

8f7bda1

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix template

e430d57

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix test

89313b3

WIP

999b0f0

Use impl trait for tuples

f87a8e4

Simplify

f3f76b6

muharem self-requested a review July 13, 2023 11:28

Ank4n mentioned this pull request Jul 17, 2023

Introduce Payees and PayoutDestination with Split variant #14451

Open

7 tasks

kianenigma self-requested a review July 20, 2023 14:03

wischli mentioned this pull request Aug 11, 2023

Generic nuke migrations centrifuge/centrifuge-chain#1492

Merged

liamaharon mentioned this pull request Aug 14, 2023

Dry-run multi-block migrations paritytech/try-runtime-cli#17

Open

ggwpez mentioned this pull request Oct 4, 2023

Multi-Block-Migrations, poll hook and new System callbacks paritytech/polkadot-sdk#1781

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FRAME] Simple multi block migrations #14275

[FRAME] Simple multi block migrations #14275

ggwpez commented May 31, 2023 •

edited

Loading

kianenigma May 31, 2023

ggwpez Jun 1, 2023 •

edited

Loading

kianenigma left a comment

liamaharon commented May 31, 2023 •

edited

Loading

xlc commented May 31, 2023

ggwpez commented Jun 1, 2023

kianenigma commented Jun 1, 2023

paritytech-cicd-pr commented Jun 29, 2023

paritytech-cicd-pr commented Jun 29, 2023

kianenigma commented Jul 20, 2023

ggwpez commented Jul 20, 2023

[FRAME] Simple multi block migrations #14275

Are you sure you want to change the base?

[FRAME] Simple multi block migrations #14275

Conversation

ggwpez commented May 31, 2023 • edited Loading

pallet-migrations

frame-executive + BlockBuilder

Runtime API

frame-support

TODO

kianenigma May 31, 2023

Choose a reason for hiding this comment

ggwpez Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

liamaharon commented May 31, 2023 • edited Loading

xlc commented May 31, 2023

ggwpez commented Jun 1, 2023

kianenigma commented Jun 1, 2023

paritytech-cicd-pr commented Jun 29, 2023

paritytech-cicd-pr commented Jun 29, 2023

kianenigma commented Jul 20, 2023

ggwpez commented Jul 20, 2023

ggwpez commented May 31, 2023 •

edited

Loading

`pallet-migrations`

`frame-executive` + `BlockBuilder`

`frame-support`

ggwpez Jun 1, 2023 •

edited

Loading

liamaharon commented May 31, 2023 •

edited

Loading